Add netsim_args to cvd load config and plumb to launcher#2648
Conversation
- Update load_config.proto to include netsim_args in EnvironmentSpecification and EnvironmentOptions. - Update launch_cvd_parser.cpp to generate --netsim_args flag from the proto. - Regenerate Go proto code. - Add unit test to flags_parser_test.cc. TAG=agy CONV=1de71887-825c-459d-a0c4-0168b1189286
| optional Common common = 4; | ||
| optional bool netsim_bt = 5; | ||
| optional bool netsim_uwb = 6; | ||
| optional string netsim_args = 7; |
There was a problem hiding this comment.
This might make more sense as a repeated string netsim_args, so that we don't have to deal with users trying to escape whitespace in this string.
Unfortunately the internal --netsim_args argument to assemble_cvd already is based on naive string splitting. This is an implementation detail we can fix later, but we should aspire to have a more sensible public API as it will be harder to change later. To make the behavior more predictable, the launch_cvd_parser.cpp implementation should introduce the whitespace to create the netsim_args flag, and error if the user-provided value has any whitespace. We can lift that restriction later.
Then again, this probably makes life slightly more difficult for all the callers specifying override flags for netsim_args...
Same concern applies for EnvironmentOptions below.
| EXPECT_TRUE(ParseJsonString(json_text, json_configs)) | ||
| << "Invalid Json string"; | ||
| auto serialized_data = LaunchCvdParserTester(json_configs); | ||
| EXPECT_TRUE(serialized_data.ok()) << serialized_data.error().Trace(); |
There was a problem hiding this comment.
nit: this can use EXPECT_THAT(serialized_data, IsOk())
android-cuttlefish/base/cvd/cuttlefish/result/result_matchers.h
Lines 27 to 35 in 4f1f0a7
| optional Common common = 4; | ||
| optional bool netsim_bt = 5; | ||
| optional bool netsim_uwb = 6; | ||
| optional string netsim_args = 7; |
There was a problem hiding this comment.
Also came across go/netsim-args-oxygen which covers a time we talked about this before. I vaguely remember that either @jemoreira or I made the argument that exposing netsim_args opened the possibility of mutually contradictory config settings, e.g.
{
"netsim_bt": false,
"netsim_args": "-bt=true"
}(not that any such -bt=true flag exists)
There was a problem hiding this comment.
It was actually Ram's position, we were just the ones in the meeting because of different time zones.
4 participants
Generated by Jetski
Bug: b/447440168